Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @jrobble)

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

    job_properties: Mapping[str, str]
    media_properties: Mapping[str, str]
    feed_forward_tracks: Optional[List[VideoTrack]] = None

Shouldn't this job type always have feed_forward_tracks set? If so, the field should not be Optional and should not have a default value.

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

        if video_job.feed_forward_track is not None:
            video_job.feed_forward_track.detection_properties['annotated_prop_single_track'] = 'annotated_val_single_track'

These properties are only ever set, not read. These changes look like they were for debugging or you intended to update a unit test, but never did.

Reviewable status: 0 of 8 files reviewed, 2 unresolved discussions (waiting on @brosenberg42 and @jrobble)

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

According to this logic we never create a feed-forward all tracks request if there are no feed-forward tracks generated in the previous task:

private Optional<DetectionRequest> createFeedForwardAllTracksRequest(Media media, DetectionContext context) {
        int topQualityCount = getTopQualityCount(context);
        String topQualitySelectionProp = context.getQualitySelectionProperty();
        var tracks = _triggerProcessor.getTriggeredTracks(media, context)
                .filter(t -> {
                    if (t.getDetections().isEmpty()) {
                        log.warn("Found track with no detections. "
                                    + "No feed forward request will be created for: {}", t);
                        return false;
                    }
                    return true;
                })
                .collect(Collectors.toList());

        if (tracks.isEmpty()) {
            return Optional.empty();
        }

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

The reason I set the properties you're referring to is to show the difference between feed-forward all tracks and feed-forward single tracks in the JSON output object when running this component in a Docker deployment with the WFM.

It appears that this component is only used as a developer reference. Technically, none of the properties are ever read.

This component is very similar to openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_python_components/test_component.py, which is used by openmpf-projects/openmpf/trunk/detection/executor/cpp/batch/test/test_batch_executor.cpp. Properties in that test_component.py are read by those tests.

I updated the test_component.py used by the test_batch_executor.cpp in order to test feed-forward all tracks behavior.

Is your desire to keep the two test_component.py files nearly identical?

@brosenberg42 reviewed 8 of 8 files at r2.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @jrobble)

detection/api/mpf_component_api/mpf_component_api.py line 83 at r2 (raw file):

Ok, please change feed_forward_tracks: Optional[List[VideoTrack]] = None to feed_forward_tracks: List[VideoTrack]

detection/examples/PythonTestComponent/test_component/test_component.py line 74 at r2 (raw file):

Part of the reason I made the initial comment was because I wanted you to add a unit test for the all tracks behavior. I do not see a reason for the two to be identical. If you would like to leave the properties here, that is fine.